Fix OOM during production webpack build#355
Fix OOM during production webpack build#355upalatucci wants to merge 1 commit intoopenshift:mainfrom
Conversation
Set NODE_OPTIONS=--max-old-space-size=4096 for the build script to prevent OOM kills in CI, matching the pattern already used by the dev script. Co-authored-by: Cursor <cursoragent@cursor.com>
WalkthroughThe build script in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 9: The dev npm script currently scopes NODE_OPTIONS only to the rm
command because the env assignment precedes rm -rf dist; update the "dev" script
so NODE_OPTIONS is applied to the webpack serve invocation (the entire chain) by
moving the env assignment to prefix the npm run ts-node invocation (i.e., ensure
NODE_ENV=development and NODE_OPTIONS=--max-old-space-size=8192 are set for npm
run ts-node -- ./node_modules/.bin/webpack serve) or alternatively use a
cross-platform tool like cross-env to export NODE_OPTIONS for the webpack serve
process; modify the "dev" script string accordingly so the webpack/ts-node
process receives the 8192 MB heap limit.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
package.json
| @@ -7,7 +7,7 @@ | |||
| "scripts": { | |||
| "clean": "rm -rf dist", | |||
| "dev": "NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 rm -rf dist && npm run ts-node -- ./node_modules/.bin/webpack serve --progress", | |||
There was a problem hiding this comment.
Pre-existing bug: NODE_OPTIONS in dev does not reach webpack.
NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 rm -rf dist && npm run ts-node -- ...webpack serve
In POSIX shell, KEY=val cmd1 && cmd2 scopes the env var to cmd1 only. Here NODE_OPTIONS=--max-old-space-size=8192 is scoped to rm -rf dist (not a Node.js process) and is silently ignored; it never reaches the npm run ts-node invocation. The webpack serve process in dev has been running without the intended 8192 MB heap limit.
The fix mirrors the corrected pattern from line 10:
🐛 Proposed fix
-"dev": "NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 rm -rf dist && npm run ts-node -- ./node_modules/.bin/webpack serve --progress",
+"dev": "rm -rf dist && NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 npm run ts-node -- ./node_modules/.bin/webpack serve --progress",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 rm -rf dist && npm run ts-node -- ./node_modules/.bin/webpack serve --progress", | |
| "dev": "rm -rf dist && NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 npm run ts-node -- ./node_modules/.bin/webpack serve --progress", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 9, The dev npm script currently scopes NODE_OPTIONS
only to the rm command because the env assignment precedes rm -rf dist; update
the "dev" script so NODE_OPTIONS is applied to the webpack serve invocation (the
entire chain) by moving the env assignment to prefix the npm run ts-node
invocation (i.e., ensure NODE_ENV=development and
NODE_OPTIONS=--max-old-space-size=8192 are set for npm run ts-node --
./node_modules/.bin/webpack serve) or alternatively use a cross-platform tool
like cross-env to export NODE_OPTIONS for the webpack serve process; modify the
"dev" script string accordingly so the webpack/ts-node process receives the 8192
MB heap limit.
There was a problem hiding this comment.
We should change this to npm run clean && NODE_ENV=development NODE_OPTIONS=--max-old-space-size=8192 npm run ts-node -- ./node_modules/.bin/webpack serve --progress
There was a problem hiding this comment.
@adamviktora Excellent suggestion! Using npm run clean instead of rm -rf dist is indeed better because it:
- Maintains consistency with other scripts like
buildandbuild-devwhich already usenpm run clean - Provides better abstraction – if the clean logic needs to change in the future, it only needs to be updated in one place
- Still fixes the NODE_OPTIONS scoping issue by placing the environment variables before the
npm run ts-nodeinvocation
Your proposed change properly ensures that both NODE_ENV and NODE_OPTIONS are scoped to the webpack serve process while keeping the script structure clean and maintainable.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: upalatucci The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@upalatucci: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
imagesjob can fail withSIGKILL(OOM kill) during the webpack production build because the process exceeds the container memory limit.NODE_OPTIONS=--max-old-space-size=4096in thebuildscript, matching the pattern already used by thedevscript (which sets 8192MB).Test plan
imagesjob should pass without OOMMade with Cursor
Summary by CodeRabbit